Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ensure StakingBalance is shown or hidden appropriately per asset… #12987

Conversation

nickewansmith
Copy link
Contributor

@nickewansmith nickewansmith commented Jan 14, 2025

Description

  1. What is the reason for the change?

There are 2 separate issues that are both related to the StakedBalance component. One is Asset Detail StakingBalance shows for ETH assets on unsupported chains with popular networks filter. The other is Asset Detail StakingBalance does not show for mainnet ETH asset when on unsupported chain with popular networks filter.

  1. What is the improvement/solution?

The user will not see staked ethereum info for assets that are not mainnet Ethereum asset when on mainnet

The user will see staked ethereum info when on unsupported chain with popular networks filter and viewing the mainnet Ethereum asset detail. We show the details but hide any staking actions for the time being while we re-evaluate how to keep the actions and switch the network on the fly.

Related issues

Fixes: https://consensyssoftware.atlassian.net/browse/STAKE-921

Manual testing steps

Asset Detail StakingBalance shows for ETH assets on unsupported chains with popular networks filter

  • Switch to mainnet network

  • Switch to popular networks filter

  • Select the native asset of a popular network that is not mainnet, such as Base or Optimism

  • In the asset detail page you should not see staked ethereum info

Asset Detail StakingBalance does not show for mainnet ETH asset when on unsupported chain with popular networks filter

Switch to Base network

Switch to popular networks filter

Select the mainnet Ethereum asset

In the asset detail page you should see staked ethereum info and earnings info but no action buttons or claim link banners

Screenshots/Recordings

Before

Screen.Recording.2025-01-15.at.8.19.49.AM.mov

After

Simulator.Screen.Recording.-.iPhone.SE.3rd.generation.-.2025-01-15.at.08.18.44.mp4

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@nickewansmith nickewansmith added needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. Run Smoke E2E Triggers smoke e2e on Bitrise labels Jan 15, 2025
Copy link
Contributor

github-actions bot commented Jan 15, 2025

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: a30b360
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/9a923579-c2d4-43c8-8060-bf655b9dfd56

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@nickewansmith nickewansmith marked this pull request as ready for review January 15, 2025 14:21
@nickewansmith nickewansmith requested review from a team as code owners January 15, 2025 14:21
@nickewansmith nickewansmith force-pushed the STAKE-921-mobile-asset-detail-staking-balance-shows-for-eth-assets-on-unsupported-chains-with-popular-networks-filter branch from a551ef7 to f3e049d Compare January 15, 2025 14:34
@nickewansmith nickewansmith added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Jan 15, 2025
Copy link
Contributor

github-actions bot commented Jan 15, 2025

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: f3e049d
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/beb675ff-6ad4-422c-883a-10a5f5af5186

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Copy link
Contributor

@Matt561 Matt561 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've successfully tested the fix locally. LGTM! 💯

@nickewansmith nickewansmith added this pull request to the merge queue Jan 15, 2025
Merged via the queue into main with commit eef70e5 Jan 15, 2025
41 checks passed
@nickewansmith nickewansmith deleted the STAKE-921-mobile-asset-detail-staking-balance-shows-for-eth-assets-on-unsupported-chains-with-popular-networks-filter branch January 15, 2025 16:02
@github-actions github-actions bot locked and limited conversation to collaborators Jan 15, 2025
@metamaskbot metamaskbot added the release-7.39.0 Issue or pull request that will be included in release 7.39.0 label Jan 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-assets-ux-review A shared label between the Assets and UX team to flag PRs ready for consolidated team review. release-7.39.0 Issue or pull request that will be included in release 7.39.0 Run Smoke E2E Triggers smoke e2e on Bitrise team-stake
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants